Release the timeline writer lock when a trace open refuses - #2630

Verdict: PASS at ec7e28b
Written-by: Reviewer
Attestation: ec7e28b role=Reviewer

Ran at this tip:

$ cd /data/dsh/bot/workspace/rev2630/tip/plugins/timeline && TZ=UTC node --test test/store.test.js
# tests 16  # pass 16  # fail 0
$ cp /data/dsh/bot/workspace/rev2630/base/plugins/timeline/src/store.js src/store.js && TZ=UTC node --test test/store.test.js   # only the source change reverted, base main store.js
not ok 6 - a refused open releases the writer lock for the next open in this process
  error: 'the same process opens the directory again: trace /tmp/flybot-timeline-store-j1IqPA/trace is already open in this process'
# tests 16  # pass 15  # fail 1
$ cp /tmp/rev2630-tip-store.js src/store.js && cd /data/dsh/bot/workspace/rev2630/tip/plugins/timeline && DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness TZ=UTC node --import ./test/resolve.mjs --test "test/*.test.js"
# tests 154  # pass 154  # fail 0

Red first, both sides quoted above. With the base store.js blob over the tip test (the test file kept at the tip), the new case fails exactly as the issue says — trace … is already open in this process — and the tip file gives 16/16. The tip blob was hash-checked before and after the swap (3d94ce0a…), so the red is the source change. The whole timeline suite at the tip is 154/154 with the harness installed; the same suite without DSH_HARNESS fails only test/host.test.js on ERR_MODULE_NOT_FOUND @deepseek-ai/dsh-llm, an install gap in this sandbox and unrelated to the three files.

Scope. pulls/2630/files: 3 files, all in the claim — plugins/timeline/src/store.js (+117/−103), plugins/timeline/test/store.test.js (+28/−1), changelog.d/2629.fixed.md (+1). timeline.js, index.js and client.js are untouched, so the persisting=false memory fallback cannot have moved.

• Pure indentation, checked. A whitespace-insensitive line diff of base vs tip store.js yields 14 changed lines and nothing else: the four comment lines, try {, } catch (error) {, the three comment lines, if (lock) {, held.delete(dir), try { unlinkSync(lock) } catch { /* already gone */ }, throw error, and the two closing braces. fresh/newest/flush/schedule/prune are declared inside the try but only used by the returned object literal, which is created inside the same block, so their narrower scope is not observable. newest() still runs before prune(), kept is still the same array, and the persist: false path skips the same work as before.
• The catch releases and rethrows. store.js:216-223: if (lock) { held.delete(dir); try { unlinkSync(lock) } catch { /* already gone */ } } throw error. The unlink is best-effort like close(), and the original error is rethrown unchanged — no in-memory trace is returned, so timeline.js still sees the failure and falls back. lock is only set once acquire() returned, so an acquire() that refuses leaves held untouched.
• Success still holds the lock to close(). acquire(), traceExists() and the writer.lock content (<pid>\n) are byte-identical between base and tip; close() is unchanged; the success path takes no early unlink. Existing tests 4-5 (a live holder is respected, a dead one is taken over, a second recorder in this process is refused) still pass, so the second same-process openTrace is still refused with TraceLocked.
• The new test is not widened. The test diff is one import (chmodSync) plus the new case; no existing assertion changed. It asserts the three things that matter: EACCES is thrown, the same-process reopen succeeds, and writer.lock is gone from disk (store.test.js:96-117).
• Checkpoint and self-report. pr 2630, claim Dev (= Owner:), issue 2629 (= Fixes #2629), 3 files all in the diff, checks.at is this 40-char tip, summary 398 ≤ 600, block 815 ≤ 1500, next 23 chars. Docs-not-needed: is in the PR body. The body's Ran: list matches what I see: 16/16 store, 154/154 timeline.
• CI at this tip: tests-ok green; Test, Merge gate, plugins (timeline), package, Node 24 · DSH 0.2.0-rc.2 all success; the only red is enqueue-guard, refusing for no Verifier report names the tip ec7e28beded0 — this comment is that report. Tier from the risk-tier job summary: T2.

Not run: root npm run verify (this box has no git, and arch-docs needs git log), the core suite (not touched by this diff), and the smoke test (no core/index.js / core/src/host/ change).